Repository navigation
enqueue-guard: read every clause before the lift, not just one - #2293
Merged
Merged
Conversation
P-491: the clause immediately before the lift was the only one read, so a
condition two clauses back was dropped and an answer that withholds the
lift cleared the hold ("If the second reader signs off. CI is green. You
may merge this tip now."). The earlier clauses are read with the opening
set, without the bare negation this rule carries for the clause right next
to the lift, so "I don't see any objections. Tests are green." still
clears.
Fixes ThinkFlowLab#2291
Product's review found the far read refused three ordinary yeses — "After the fix, …", "It should be fine. …", "Following the merge, …" — because a time or sequencing phrase before the yes was read as a condition on it. The clause next to the lift and the clauses further back now share one vocabulary of waiting and condition words, so the two positions cannot drift apart again; "following the merge" opens one of them and clears. Fixes ThinkFlowLab#2291
…further back
The clause next to the lift lost the bare negation and the temporal openings
when the far-clause vocabulary was merged into it, so five answers that merged
main reads as withheld cleared a standing hold ("The second reader has not
looked at it yet — you may merge this tip now.").
Read the clause at liftAt - 1 exactly as main does, and give only the clauses
before it the narrower set: waiting and condition words plus if/unless/until/
once, no bare negation, no time or sequencing openings. The five cases are
tests of their own, red on the previous tip.
HugoXK
marked this pull request as ready for review
November 3, 2026 11:10
Contributor
Author
|
@Birfy — review request for PR #2293 at tip
|
Collaborator
|
Verdict: PASS at dce22dc Ran at this tip: Red first, one swap: Review points
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Written-by: Dev-4
Owner: Dev-4
Reviewer: Birfy
Fixes #2291
Base: 6e5f79a
What changed
.github/scripts/enqueue-rules.mjs,unconditionalLift: the clause next to the lift is read exactly as merged main reads it, and the clauses further back get a narrower read. Main'sif (liftAt > 0 && withholdsBefore(clauses[liftAt - 1]))is unchanged; one line is added forclauses.slice(0, liftAt - 1).That split is the fix for two opposite defects at once. Reading only
liftAt - 1drops a condition with one neutral clause in between (the eight bodies below). Reading the same broad vocabulary further back refuses ordinary answers —I don't see any objections. Tests are green. You may merge this tip now.— which is the #2287 shape.The eight bodies this closes (each its own test,
#2291 g1–g8):If the second reader signs off. CI is green. You may merge this tip now.Only after the second reader re-runs — looks good. You may merge this tip.Once the second reader re-runs. Thanks. You may merge this tip.Pending the second reader. Nothing else is open. You may merge this tip now.Unless the second reader objects we wait. Looks fine. You may merge this tip now.Provided the second reader lands — I am around. You may merge this tip now.Not until the second reader re-runs. We wait. You may merge this tip now.If the second reader signs off./CI is green./you may merge this tip.(newline-separated)The far read uses the waiting and condition words (
only after,only once,only when,not until,not unless,pending,subject to,provided,providing,as long/soon as,assuming,in case,depending on,contingent on/upon,dependent on/upon,in the event,on condition that) plus an opening test ofif,unless,untilandonce(once more/once againexcluded). It carries no bare negation and no time or sequencing opening: further back,notis the answer's own voice andAfter the fix/It should be fineare ordinary prose.Five bodies that merged main reads as withheld must keep stopping next to the lift, and each is a test of its own (
#2291 k1–k5). They were cleared by the previous tip of this branch, which read the narrow vocabulary at both positions; that regression is the reason the two positions are now separate:The second reader has not looked at it yet — you may merge this tip now.The second reader hasn't signed off — you may merge this tip now.The second reader never re-ran it — you may merge this tip now.When the second reader signs off — you may merge this tip now.After the second reader re-runs — you may merge this tip now.Four ordinary yeses with a negation further back than the lift keep clearing (
#2291 n1–n4):I don't see any objections. Tests are green. You may merge this tip now.I was not able to break it. All green. You may merge this tip now.I could not find a hole in it. Ship it. You may merge this tip now.Nothing blocking from me. CI is green. You may merge this tip now.Time phrasing further back clears (
#2291 n5–n7):After the fix, all green. CI is green. You may merge this tip now.,It should be fine. Tests are green. You may merge this tip now.,Following the merge, all green. CI is green. You may merge this tip now.Known boundaries, both deliberate and both stated rather than chased:
Following the merge, all green.is the clause next to the lift, so it keeps main's read (followingwithholds there) —Following the merge, all green. You may merge this tip now.still stops.Following the second reader's sign-off — you may merge this tip now.sits in the same slot and stops on main too. Two clauses back, where the narrow vocabulary applies, the same words clear.Nothing is signed off by the second reader. CI is green. You may merge this tip now.still lifts, the same trade-off the one-clause read already made.This tip supersedes an earlier tip of this branch (v2), withdrawn before review because it also replaced the adjacent-clause word list; the change here leaves
liftAt - 1untouched and only narrows the further-back scan.Nothing else moved: no other file, no other predicate.
Ran at the tip
Red first, on live main
6e5f79ade54a89cb30016c98688ef88281158331— a clean worktree at that sha, source at that base, only the final test file added:Only the eight holes are red on that base.
k1–k5andn1–n7pass there, which is the point of them: they are the floor this fix may not dig under and the ordinary yeses it may not refuse.Regression red, on the previous tip of this branch
aa424db246a8c294a9d78c47ce86baeac60827bf, with the same test file:At this tip (Node v22.23.2,
TZ=UTC):The full-suite run uses
npm_config_cache=/home/xuke/.dsh/bot/workspace/.npmcache:plugin-importsrunsnpm pack, and this machine's~/.npmis read-only (EROFS), which fails that one test on any tip — unrelated to this diff. Main carries 633 tests there; these eleven (eight holes, the five-bodies test, the two counter loops) make 644.The acceptance probe,
probes/lift-preceding-clause-gap.mjs, counts the counter-cases and the next-to-the-lift cases in its verdict. Three hard readings, each its own run:The third reading is the guard against buying the eight holes by refusing the ordinary yeses: shape A closes all eight and is still a failure, because it refuses three of the four counter-cases.
The other probes, over this tree:
Not run: the
core/+plugins/half ofverify. The diff touches neither path, and this worktree has no installed dependencies.Tier
T3 by path:
.github/**is T3 in.github/agent/risk-rules.json, so this needs a formal conclusion naming this tip before it can enter the queue.{ "pr": 2293, "claim": "Dev-4", "issue": 2291, "summary": "A lift clears an enqueue-guard stop only when it is unconditional, and a condition can sit several clauses away from the yes. The clause next to the lift keeps merged main's reading, negation and time words included, and only the clauses further back get the narrow set of waiting and condition words with if, unless, until and once as openings. Eight withheld bodies now stop, the five bodies that read regressed on the previous tip stop again, and four ordinary yeses with a negation further back keep clearing.", "files": [ ".github/scripts/enqueue-rules.mjs", ".github/scripts/enqueue-guard.test.mjs" ], "checks": { "ran": "node --test '.github/scripts/*.test.mjs'", "result": "green", "at": "dce22dce574297f168e792b2ed43fa94137c7377" }, "next": "none — ready for review" }